-
Notifications
You must be signed in to change notification settings - Fork 211
[PROD] - UTM register btn #7168
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| if (!auth.tokenV3) { | ||
| const utmSource = communityId || 'community-app-main'; | ||
| window.location.href = `${config.URL.AUTH}/member?retUrl=${encodeURIComponent(`${window.location.origin}${window.location.pathname}`)}&utm_source=${utmSource}®Source=challenges`; | ||
| window.location.href = appendUtmParamsToUrl( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[❗❗ security]
The appendUtmParamsToUrl function is used to construct the URL with UTM parameters. Ensure that this function properly encodes all URL components to prevent any potential URL injection vulnerabilities.
|
|
||
| // handle values that might contain '=' | ||
| const cookieValue = decodeURIComponent(cookieStr.split('=').slice(1).join('=')); | ||
| return JSON.parse(cookieValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[maintainability]
Consider logging the error or providing more context in the catch block to aid in debugging if JSON parsing fails.
|
|
||
| return urlObj.toString(); | ||
| } catch (error) { | ||
| return url; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[maintainability]
Consider logging the error or providing more context in the catch block to aid in debugging if URL manipulation fails.
| - run: | ||
| name: App npm install | ||
| command: npm install | ||
| command: npm ci |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[correctness]
Switching from npm install to npm ci is a good practice for CI environments as it ensures a clean install based on the lock file. However, ensure that the package-lock.json is up-to-date and committed to the repository to avoid discrepancies between local and CI environments.
| RUN npm config set unsafe-perm true | ||
| RUN git config --global url."https://git@".insteadOf git:// | ||
| RUN npm install | ||
| RUN npm ci |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[correctness]
Switching from npm install to npm ci is a good practice for CI/CD environments as it ensures a clean install based on the package-lock.json file, leading to more consistent builds. However, ensure that the package-lock.json file is up-to-date and committed to the repository, as npm ci will fail if the file is missing or out of sync with package.json.
| --url https://circleci.com/api/v2/project/github/$CIRCLE_PROJECT_USERNAME/$CIRCLE_PROJECT_REPONAME/pipeline \ | ||
| --header "Circle-Token: ${CIRCLE_TOKEN}" \ | ||
| --header 'content-type: application/json' \ | ||
| --data '{"branch":"'"$CIRCLE_BRANCH"'","parameters":{"run_smoketesting":true , "run_performancetesting":false, "run_basedeployment": false}}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[❗❗ security]
The curl command uses the Circle-Token in the header, which is sensitive information. Ensure that this token is stored securely and not exposed in logs or error messages. Consider using environment variables or CircleCI's built-in secrets management to handle sensitive data.
https://topcoder.atlassian.net/browse/PM-3204